Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix occ maintenance:install database connect failure #19303

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Feb 5, 2020

Fix #18513
Fix #15560

@nickvergessen nickvergessen added the 2. developing Work in progress label Feb 5, 2020
@nickvergessen nickvergessen added this to the Nextcloud 19 milestone Feb 5, 2020
@nickvergessen nickvergessen self-assigned this Feb 5, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Feb 5, 2020

Those gotta be the best commits I've ever seen 😁
image

Provide the auth method for MySQL 8.0+

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
More gebuging

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen marked this pull request as ready for review February 5, 2020 12:56
@nickvergessen nickvergessen added 3. to review Waiting for reviews bug feature: install and update and removed 2. developing Work in progress labels Feb 5, 2020
…orrectly

Signed-off-by: Joas Schilling <coding@schilljs.com>
@kesselb

This comment has been minimized.

@skjnldsv skjnldsv changed the title Test/debug install Fix occ maintenance:install database connect failure Feb 5, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Feb 5, 2020

Failures unrelated

@skjnldsv

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@skjnldsv skjnldsv requested a review from juliusknorr February 5, 2020 14:39
@nickvergessen
Copy link
Member Author

nickvergessen commented Feb 5, 2020

Marking offtopic hides the comments only, or aborts the backport? The commits 1+4 should be backported until 16. 2+3 could be skipped, as they just make debugging easier as it makes handling of exceptions more explicit, but they don't fix a primary error.

@skjnldsv
Copy link
Member

skjnldsv commented Feb 5, 2020

Marking offtopic hides the comments only, or aborts the backport?

only hide :)
This was for visibility 😉

@MichaIng
Copy link
Member

MichaIng commented Feb 5, 2020

Okay this fixes the authentication plugin situation currently, but e.g. requires maintenance when new MySQL version come up or MariaDB switches to a different default authentication method, when a password is supplied that way.

An alternative that would be more future prove:

$query = "CREATE USER '$name'@'localhost' IDENTIFIED WITH mysql_native_password;"
$connection->executeUpdate($query);
$query = "ALTER USER '$name'@'localhost' IDENTIFIED BY '$password';"
$connection->executeUpdate($query);

@nickvergessen
Copy link
Member Author

I checked that and it didn't work on mysql 8 and mariadb 10.4 with the same Syntax. Password needs to be hashed manually for mariadb etc. So i think that's the better story for now

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 6, 2020
@nickvergessen nickvergessen merged commit c5c3178 into master Feb 6, 2020
@nickvergessen nickvergessen deleted the test/debug-install branch February 6, 2020 18:36
@backportbot-nextcloud
Copy link

backport to stable18 in #19326

@nickvergessen
Copy link
Member Author

/backport to stable17

@nickvergessen
Copy link
Member Author

/backport to stable16

@backportbot-nextcloud
Copy link

backport to stable17 in #19327

@backportbot-nextcloud
Copy link

backport to stable16 in #19328

@MichaIng
Copy link
Member

MichaIng commented Feb 6, 2020

@nickvergessen

I checked that and it didn't work on mysql 8 and mariadb 10.4 with the same Syntax. Password needs to be hashed manually for mariadb etc. So i think that's the better story for now

Strange, works perfectly fine here with MariaDB:

2020-02-06 23:28:45 root@VM-Buster:~$ mariadb
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 47
Server version: 10.3.18-MariaDB-0+deb10u1 Debian 10

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+----------+
| user | host      | plugin      | password |
+------+-----------+-------------+----------+
| root | localhost | unix_socket |          |
+------+-----------+-------------+----------+
1 row in set (0.000 sec)

MariaDB [(none)]> create user test@localhost identified with mysql_native_password;
Query OK, 0 rows affected (0.001 sec)

MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+----------+
| user | host      | plugin      | password |
+------+-----------+-------------+----------+
| root | localhost | unix_socket |          |
| test | localhost |             |          |
+------+-----------+-------------+----------+
2 rows in set (0.000 sec)

MariaDB [(none)]> alter user test@localhost identified by 'test';
Query OK, 0 rows affected (0.001 sec)

MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+-------------------------------------------+
| user | host      | plugin      | password                                  |
+------+-----------+-------------+-------------------------------------------+
| root | localhost | unix_socket |                                           |
| test | localhost |             | *94BDCEBE19083CE2A1F959FD02F964C7AF4CFC29 |
+------+-----------+-------------+-------------------------------------------+
2 rows in set (0.000 sec)

Also the docs state clearly that plain test password must be used here: https://mariadb.com/kb/en/alter-user/
Hashed password is possible/required when using:

... IDENTIFIED BY PASSWORD 'password_hash'

Such a solution would solve the issue with Nextcloud 16 as well: #19328 (comment)

Sadly Debian does not ship MariaDB even on Bullseye+Sid and no MySQL, hence I cannot test with these.

@nickvergessen
Copy link
Member Author

Well yes it works like that, but that doesn't work with mysql8. And if you specify the plugin it requires the hash in mariadb. Anyway the PR we use now fixes it for all supported dbs... Good enough

@MichaIng
Copy link
Member

MichaIng commented Feb 7, 2020

@nickvergessen

And if you specify the plugin it requires the hash in mariadb.

That must be either wrong or new in MariaDB 10.4, as with 10.3 it's not true, as of the two different syntax for plain and hashed password, see above.

but that doesn't work with mysql8

Okay, no chance then. A pain that syntax differs on such basic SQL commands 😢.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: install and update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[17.0.2] "occ maintenance:install" uses "oc_<admin>" before creating it mysql port syntax in config
5 participants